refactor: Don't modify ContainerBuilder after it has been build by xeniape · Pull Request #838 · stackabletech/nifi-operator

xeniape · 2025-08-26T14:26:47Z

Description

During reconciliation the moved code from the PR was called after pod_builder.add_container(container_nifi.build()), therefore not updating the volumeMounts of the nifi container with the volumeMounts related to the authentication config and they were missing. This PR adds them in.

After talking with @sbernauer: The prepare container was configuring things with the auth configs being mounted, so the code/tests still worked as expected.

But hence the volumeMounts are not needed on the nifi container (which weren't applied anyway due to the crossed out explanation above). The correct way would be to remove the ContainerBuilder from the mounting step.

Adjusted the code to reflect that, also renamed and dropped the ContainerBuilder for code clarity. Will run integrationtests to make sure everything works and add them into the description.

Integrationtests

--- FAIL: kuttl (4999.48s)
    --- FAIL: kuttl/harness (0.00s)
        --- PASS: kuttl/harness/orphaned_resources_nifi-1.27.0_zookeeper-latest-3.9.3_openshift-false (233.71s)
        --- PASS: kuttl/harness/upgrade_nifi_old-1.27.0_nifi_new-2.4.0_zookeeper-latest-3.9.3_openshift-false (413.86s)
        --- PASS: kuttl/harness/oidc-opa_nifi-1.28.1_zookeeper-latest-3.9.3_opa-l-1.4.2_oidc-use-tls-true_openshift-false (230.47s)
        --- PASS: kuttl/harness/resources_nifi-2.4.0_zookeeper-latest-3.9.3_openshift-false (79.36s)
        --- PASS: kuttl/harness/resources_nifi-1.28.1_zookeeper-latest-3.9.3_openshift-false (195.59s)
        --- PASS: kuttl/harness/resources_nifi-1.27.0_zookeeper-latest-3.9.3_openshift-false (206.26s)
        --- PASS: kuttl/harness/smoke_v1_nifi-v1-1.28.1_zookeeper-3.9.3_openshift-false_listener-class-external-unstable (260.61s)
        --- PASS: kuttl/harness/smoke_v1_nifi-v1-1.28.1_zookeeper-3.9.3_openshift-false_listener-class-cluster-internal (382.25s)
        --- PASS: kuttl/harness/smoke_v1_nifi-v1-1.27.0_zookeeper-3.9.3_openshift-false_listener-class-external-unstable (379.22s)
        --- PASS: kuttl/harness/oidc-opa_nifi-2.4.0_zookeeper-latest-3.9.3_opa-l-1.4.2_oidc-use-tls-true_openshift-false (170.94s)
        --- PASS: kuttl/harness/oidc-opa_nifi-2.4.0_zookeeper-latest-3.9.3_opa-l-1.4.2_oidc-use-tls-false_openshift-false (187.98s)
        --- PASS: kuttl/harness/custom-components-git-sync_nifi-2.4.0_zookeeper-latest-3.9.3_openshift-false (164.27s)
        --- PASS: kuttl/harness/smoke_v2_nifi-v2-2.4.0_use-zookeeper-manager-true_zookeeper-3.9.3_openshift-false_listener-class-external-unstable (176.55s)
        --- PASS: kuttl/harness/smoke_v2_nifi-v2-2.4.0_use-zookeeper-manager-true_zookeeper-3.9.3_openshift-false_listener-class-cluster-internal (293.72s)
        --- FAIL: kuttl/harness/smoke_v1_nifi-v1-1.27.0_zookeeper-3.9.3_openshift-false_listener-class-cluster-internal (1286.15s)
        --- PASS: kuttl/harness/smoke_v2_nifi-v2-2.4.0_use-zookeeper-manager-false_zookeeper-3.9.3_openshift-false_listener-class-external-unstable (176.24s)
        --- PASS: kuttl/harness/smoke_v2_nifi-v2-2.4.0_use-zookeeper-manager-false_zookeeper-3.9.3_openshift-false_listener-class-cluster-internal (164.81s)
        --- PASS: kuttl/harness/external-access_nifi-2.4.0_zookeeper-latest-3.9.3_openshift-false (72.06s)
        --- FAIL: kuttl/harness/external-access_nifi-1.28.1_zookeeper-latest-3.9.3_openshift-false (386.51s)
        --- FAIL: kuttl/harness/external-access_nifi-1.27.0_zookeeper-latest-3.9.3_openshift-false (391.46s)
        --- PASS: kuttl/harness/cluster_operation_nifi-latest-2.4.0_zookeeper-latest-3.9.3_openshift-false (101.90s)
        --- PASS: kuttl/harness/logging_nifi-1.27.0_zookeeper-latest-3.9.3_openshift-false (179.48s)
        --- PASS: kuttl/harness/custom-components-git-sync_nifi-1.28.1_zookeeper-latest-3.9.3_openshift-false (170.39s)
        --- PASS: kuttl/harness/custom-components-git-sync_nifi-1.27.0_zookeeper-latest-3.9.3_openshift-false (182.79s)
        --- PASS: kuttl/harness/logging_nifi-2.4.0_zookeeper-latest-3.9.3_openshift-false (106.74s)
        --- PASS: kuttl/harness/iceberg_nifi-iceberg-2.4.0_zookeeper-latest-3.9.3_opa-l-1.4.2_hdfs-l-3.4.1_hive-l-4.0.1_trino-l-476_krb5-1.21.1_iceberg-use-kerberos-false_kerberos-realm-PROD.MYCORP_openshift-false (570.09s)
        --- PASS: kuttl/harness/iceberg_nifi-iceberg-2.4.0_zookeeper-latest-3.9.3_opa-l-1.4.2_hdfs-l-3.4.1_hive-l-4.0.1_trino-l-476_krb5-1.21.1_iceberg-use-kerberos-true_kerberos-realm-PROD.MYCORP_openshift-false (396.09s)
        --- PASS: kuttl/harness/orphaned_resources_nifi-2.4.0_zookeeper-latest-3.9.3_openshift-false (82.99s)
        --- PASS: kuttl/harness/orphaned_resources_nifi-1.28.1_zookeeper-latest-3.9.3_openshift-false (215.82s)
        --- PASS: kuttl/harness/ldap_nifi-2.4.0_zookeeper-latest-3.9.3_ldap-use-tls-false_openshift-false (226.82s)
        --- PASS: kuttl/harness/oidc-opa_nifi-1.28.1_zookeeper-latest-3.9.3_opa-l-1.4.2_oidc-use-tls-false_openshift-false (191.28s)
        --- PASS: kuttl/harness/logging_nifi-1.28.1_zookeeper-latest-3.9.3_openshift-false (153.87s)
        --- PASS: kuttl/harness/oidc-opa_nifi-1.27.0_zookeeper-latest-3.9.3_opa-l-1.4.2_oidc-use-tls-true_openshift-false (199.17s)
        --- PASS: kuttl/harness/oidc-opa_nifi-1.27.0_zookeeper-latest-3.9.3_opa-l-1.4.2_oidc-use-tls-false_openshift-false (181.15s)
        --- PASS: kuttl/harness/ldap_nifi-1.28.1_zookeeper-latest-3.9.3_ldap-use-tls-false_openshift-false (210.33s)
        --- PASS: kuttl/harness/ldap_nifi-2.4.0_zookeeper-latest-3.9.3_ldap-use-tls-true_openshift-false (258.89s)
        --- PASS: kuttl/harness/ldap_nifi-1.28.1_zookeeper-latest-3.9.3_ldap-use-tls-true_openshift-false (188.57s)
        --- PASS: kuttl/harness/ldap_nifi-1.27.0_zookeeper-latest-3.9.3_ldap-use-tls-false_openshift-false (189.44s)
        --- PASS: kuttl/harness/ldap_nifi-1.27.0_zookeeper-latest-3.9.3_ldap-use-tls-true_openshift-false (194.16s)
FAIL

--- PASS: kuttl (359.32s)
    --- PASS: kuttl/harness (0.00s)
        --- PASS: kuttl/harness/smoke_v1_nifi-v1-1.27.0_zookeeper-3.9.3_openshift-false_listener-class-cluster-internal (359.31s)
PASS

    logger.go:42: 08:48:13 | external-access_nifi-1.28.1_zookeeper-latest-3.9.3_openshift-false/30-install-nifi | nificluster.nifi.stackable.tech/test-nifi condition met
    logger.go:42: 08:48:13 | external-access_nifi-1.28.1_zookeeper-latest-3.9.3_openshift-false/30-install-nifi | test step completed 30-install-nifi
    logger.go:42: 08:48:13 | external-access_nifi-1.28.1_zookeeper-latest-3.9.3_openshift-false | skipping kubernetes event logging
    logger.go:42: 08:48:13 | external-access_nifi-1.28.1_zookeeper-latest-3.9.3_openshift-false | Deleting namespace: kuttl-test-stunning-pelican
    case.go:118: context deadline exceeded

    logger.go:42: 08:56:16 | external-access_nifi-1.27.0_zookeeper-latest-3.9.3_openshift-false/30-install-nifi | nificluster.nifi.stackable.tech/test-nifi condition met
    logger.go:42: 08:56:16 | external-access_nifi-1.27.0_zookeeper-latest-3.9.3_openshift-false/30-install-nifi | test step completed 30-install-nifi
    logger.go:42: 08:56:16 | external-access_nifi-1.27.0_zookeeper-latest-3.9.3_openshift-false | skipping kubernetes event logging
    logger.go:42: 08:56:16 | external-access_nifi-1.27.0_zookeeper-latest-3.9.3_openshift-false | Deleting namespace: kuttl-test-unbiased-marmoset
    case.go:118: context deadline exceeded

Succeeded but timed out on deleting namespace

Definition of Done Checklist

Not all of these items are applicable to all PRs, the author should update this template to only leave the boxes in that are relevant
Please make sure all these things are done and tick the boxes

Author

Changes are OpenShift compatible
CRD changes approved
CRD documentation for all fields, following the style guide.
Helm chart can be installed and deployed operator works
Integration tests passed (for non trivial changes)
Changes need to be "offline" compatible
Links to generated (nightly) docs added
Release note snippet added

Reviewer

Code contains useful comments
Code contains useful logging statements
(Integration-)Test cases added
Documentation added or updated. Follows the style guide.
Changelog updated
Cargo.toml only contains references to git tags (not specific commits or branches)

Acceptance

Feature Tracker has been updated
Proper release label has been added
Links to generated (nightly) docs added
Release note snippet added
Add type/deprecation label & add to the deprecation schedule
Add type/experimental label & add to the experimental features tracker

maltesander

Thanks, LGTM!

sbernauer · 2025-08-27T11:49:51Z

Do we know why the ldap integration test is green besides this bug?
Can we somehow improve the test, so that it would have cached this?

xeniape · 2025-08-27T14:34:43Z

After talking with @sbernauer: The prepare container was configuring things with the auth configs being mounted, so the code/tests still worked as expected.

But hence the volumeMounts are not needed on the nifi container (which weren't applied anyway due to the explanation in the description). The correct way would be to remove the ContainerBuilder from the mounting step.

Adjusted the code to reflect that, also renamed and dropped the ContainerBuilder for code clarity. Will run integrationtests to make sure everything works and add them into the description.

sbernauer

Change itself LGTM, thanks!

rust/operator-binary/src/controller.rs

Co-authored-by: Sebastian Bernauer <sebastian.bernauer@stackable.de>

Cargo.toml

fix: propagate auth volumeMounts to nifi container

b2ebb6e

xeniape self-assigned this Aug 26, 2025

maltesander previously approved these changes Aug 27, 2025

View reviewed changes

remove nifi container from auth config mount

1e64189

xeniape dismissed maltesander’s stale review via 1e64189 August 27, 2025 14:24

move code to previous place

8e50306

sbernauer requested changes Aug 28, 2025

View reviewed changes

rust/operator-binary/src/controller.rs Show resolved Hide resolved

rust/operator-binary/src/controller.rs Show resolved Hide resolved

rust/operator-binary/src/controller.rs Outdated Show resolved Hide resolved

xeniape changed the title ~~fix: propagate auth volumeMounts to nifi container~~ refactor: Don't modify ContainerBuilder after it has been build Aug 28, 2025

xeniape and others added 2 commits August 28, 2025 09:22

bump url crate version

6edbf7f

Update rust/operator-binary/src/controller.rs

09121e5

Co-authored-by: Sebastian Bernauer <sebastian.bernauer@stackable.de>

xeniape added this to Stackable Engineering Aug 28, 2025

xeniape moved this to Development: Waiting for Review in Stackable Engineering Aug 28, 2025

xeniape requested review from maltesander and sbernauer August 28, 2025 07:34

sbernauer approved these changes Aug 28, 2025

View reviewed changes

Cargo.toml Show resolved Hide resolved

sbernauer moved this from Development: Waiting for Review to Development: In Review in Stackable Engineering Aug 28, 2025

xeniape added this pull request to the merge queue Aug 28, 2025

Merged via the queue into main with commit 3f28598 Aug 28, 2025
17 checks passed

xeniape deleted the fix/propagate-auth-volumemounts-to-nifi-container branch August 28, 2025 07:45

sbernauer moved this from Development: In Review to Development: Done in Stackable Engineering Aug 28, 2025

lfrancke added the release/25.11.0 label Sep 8, 2025

lfrancke moved this from Development: Done to Done in Stackable Engineering Sep 8, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor: Don't modify ContainerBuilder after it has been build#838

refactor: Don't modify ContainerBuilder after it has been build#838
xeniape merged 5 commits intomainfrom
fix/propagate-auth-volumemounts-to-nifi-container

xeniape commented Aug 26, 2025 •

edited

Loading

Uh oh!

maltesander left a comment

Uh oh!

sbernauer commented Aug 27, 2025

Uh oh!

xeniape commented Aug 27, 2025 •

edited

Loading

Uh oh!

sbernauer left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

xeniape commented Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Definition of Done Checklist

Author

Reviewer

Acceptance

Uh oh!

maltesander left a comment

Choose a reason for hiding this comment

Uh oh!

sbernauer commented Aug 27, 2025

Uh oh!

xeniape commented Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbernauer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

xeniape commented Aug 26, 2025 •

edited

Loading

xeniape commented Aug 27, 2025 •

edited

Loading